⚠️ GitHub issue #47659 has been automatically assigned in GitHub to PR creator.

Hi @kou and @lidavidm, this PR is ready for review, it is a small change extracted from #46099. Our team tripped on this when developing #46099 and found a fix for this.

Can we solve this by the following?

diff --git a/cpp/src/arrow/flight/test_definitions.cc b/cpp/src/arrow/flight/test_definitions.cc
index c6b8e2b422..5979dd76eb 100644
--- a/cpp/src/arrow/flight/test_definitions.cc
+++ b/cpp/src/arrow/flight/test_definitions.cc
@@ -39,7 +39,6 @@
 #include "arrow/util/checked_cast.h"
 #include "arrow/util/config.h"
 #include "arrow/util/logging.h"
-#include "gmock/gmock.h"
 
 #if defined(ARROW_CUDA)
 #  include "arrow/gpu/cuda_api.h"

Hi @kou, I have tried out the solution and the errors came back after undoing the changes at cpp\src\arrow\flight\CMakeLists.txt. And it seems the include of gmock at test_definitions.cc is not necessary and can be removed, so I removed it.

I then found there is also #include <gmock/gmock.h> at
cpp\src\arrow\flight\test_util.h. And after trying to remove the gmock.h reference at test_util.h, I found that the mock include is required for the file test_util.h.

Ah, OK.

Could you use SHARED_LINK_LIBS not SHARED_PRIVATE_LINK_LIBS? GMock must be a public dependency because it's used in .h not .cc.

Isn't the gmock dependency already satisfied through the use of ARROW_FLIGHT_TEST_LINK_LIBS? One thing that's a little risky in providing GMOCK as a public dependency is that when it is combined with GTEST_MAIN on Windows it has the very surprising behavior of causing tests to not run at all (see #47434)

Ah, should we move

Worth a shot!

@kou I have moved AssertEqual definition to test_util.cc and checked that gmock/gmock.h is not included in any *.h test files. But the 12 errors still occur without gmock dependency being added. I found that arrow_testing lib uses gmock links directly (here) so I went with this approach and used SHARED_LINK_LIBS and removed the SHARED_PRIVATE_LINK_LIBS.

@WillAyd I found ARROW_FLIGHT_TESTING_<STATIC | SHARED>_LINK_LIBS are not linked to ARROW_TEST_<STATIC | SHARED>_LINK_LIBS. The arrow library arrow_flight_testing is linked with ARROW_FLIGHT_TESTING_<STATIC | SHARED>_LINK_LIBS, not ARROW_FLIGHT_TEST_LINK_LIBS, so we had the gmock dependency issue.
If my understanding is correct, #47434 impacts tests created by add_arrow_test more and doesn't impact libraries created by add_arrow_lib. Tests have a .exe test executable (where tests were not run) and arrow_flight_testing library gives a dll file on Windows.

Thanks both for providing suggestions and info

@WillAyd I found ARROW_FLIGHT_TESTING_<STATIC | SHARED>_LINK_LIBS are not linked to ARROW_TEST_<STATIC | SHARED>_LINK_LIBS. The arrow library arrow_flight_testing is linked with ARROW_FLIGHT_TESTING_<STATIC | SHARED>_LINK_LIBS, not ARROW_FLIGHT_TEST_LINK_LIBS, so we had the gmock dependency issue.
If my understanding is correct, #47434 impacts tests created by add_arrow_test more and doesn't impact libraries created by add_arrow_lib. Tests have a .exe test executable (where tests were not run) and arrow_flight_testing library gives a dll file on Windows.

To try and clarify, my concern was less about the library itself and more about tests that use the library. I couldn't find any definitive guidance about the different combinations of g{test|mock}_{main} and how their linkage affects the test runner on Windows. Its also easy to overlook when it causes an issue; the test suite still passes, so you'd have to manually inspect the logs to see if tests were actually being run

The main Windows CI job I don't think has FLIGHT tests enabled, so its hard to tell from the CI runs here. When you try locally, are you sure that tests are actually collected and running? If so, then I'm OK with this - I certainly don't want to go down a rabbit hole chasing issues

@WillAyd Got it, thanks for your clarification. On my branch, I have run ./arrow-flight-sql-test.exe and ./arrow-flight-test.exe on my local Windows MSVC environment and can confirm tests are being collected and run.

I am not very familiar with the combinations of gmock linkage either. Current Windows CI only run on MinGW64 and CLANG64 (that I can find), and I see on Windows CI that flight tests do seem to be run there:

arrow_flight           =   7.06 sec*proc (2 tests)
arrow_flight_sql       =   0.93 sec*proc (1 test)

I think these tests have always been running on the Windows CI because I was able to catch Flight SQL ODBC test failures using CI before the fix of #47434.
#47434 brings an important fix that resolves MSVC Windows specifically. Before #47434, my team running test executables on Windows MSVC locally would also get [ PASSED ] 0 tests., so we also added the RUN_ALL_TESTS as a temporary fix. After #47434, these tests are being run without usages of RUN_ALL_TESTS.
I don't think there is any MSVC Windows CI (for flight at least), and if any issues occur at MSVC Windows we won't be able to catch it using CI, so maybe that's the bigger concern

No problem!
@kou @lidavidm gentle poke for the PR, please let me know if you have other comments

After merging your PR, Conbench analyzed the 2 benchmarking runs that have been run so far on merge-commit bc9746d.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.